Rich Text: Avoid activeElement focus call #20594
Conversation
aduth
added
[Type] Bug
aduth
requested review from
daniloercoli,
etoledom,
jorgefilipecosta and
SergioEstevao
as code owners
|
Size Change: +32 B (0%) Total Size: 865 kB
ℹ️ View Unchanged
|
Related (reinforcing):
https://developer.mozilla.org/en-US/docs/Web/API/DocumentOrShadowRoot/activeElement |
|
An additional finding from some last-minute testing: This resolves the error by avoiding calling on a property which may be |
See: #20598 |
* Rich Text: Avoid activeElement focus call * Rich Text: Restore focus, accounting for null activeElement * Rich Text: Verify activeElement instanceof HTMLElement
* Rich Text: Avoid activeElement focus call * Rich Text: Restore focus, accounting for null activeElement * Rich Text: Verify activeElement instanceof HTMLElement
jorgefilipecosta
removed
the
Backport to WP 6.7 Beta/RC
| activeElement.focus(); | ||
| } | ||
| } else if ( document.activeElement instanceof window.HTMLElement ) { | ||
| document.activeElement.blur(); |
There was a problem hiding this comment.
@ellatrix If I recall correctly, the idea was that if this logic should not cause a change in focus, and there was no focused element before the selection applied, we should guarantee that there is no focused element after it's applied by calling blur on whichever element is focused at that point.
I'm not able to remember if there was a specific case in mind this was addressing, or if it was only here to deal with the hypothetical technical possibility.
Based on the debugging information from the original pull request comment, I suspect the main fix for Trac#49519 is largely in adding the condition of if ( activeElement ) { (accounting for the fact that activeElement can be null). If that's enough to accommodate all of the current scenarios and fix both Trac#49519 and #20598, then maybe it would be fine to remove this blur call.
Fixes: https://core.trac.wordpress.org/ticket/49519
This pull request attempts to fix Trac#49519, where an error occurs when deleting blocks in the editor in Internet Explorer.
The error manifests itself as:
It was traced back to this line of code in the
applySelectionfunction:gutenberg/packages/rich-text/src/to-dom.js
Line 305 in 1196354
This was recently changed/introduced as part of #19536.
It's not immediately clear to me whether this explicit focus is required. It may be that the manipulation of selections on the lines preceeding the
focus()call would cause focus to be lost, so an explicit focus is intended in restoring the focus which had existed before those selection manipulations occurred.Ultimately, it seems that
activeElementis unset in Internet Explorer, but not other browsers.If removing the
focusis not acceptable, the following alternatives may be considered:activeElement, to at least verify existence before callingfocusactiveElement? Specifically, I have found thatactiveElementwill tend to fall back todocument.bodyin many other browsers (see related example).range.commonAncestorContainerTesting Instructions:
In Internet Explorer and your preferred browser, verify no errors when deleting block: